Skip to content

chore(lint): Allow ts-ignore in Node integration tests #13254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Aug 6, 2024

Discovered today that TS 3.8 doesn't understand // @ts-expect-error. Since we test on Node 22 also with TS 3.8, we shouldn't use ts-expect-error in test files as TS 3.8 would simply ignore the comment and throw a type error.

Instead, it's fine to use @ts-ignore in these test files.

@Lms24 Lms24 changed the base branch from develop to lms/feat-core-metaTags August 6, 2024 12:27
Base automatically changed from lms/feat-core-metaTags to develop August 6, 2024 12:38
@Lms24 Lms24 force-pushed the lms/chore-lint-tsignore-node-integ-tests branch from 55fcf00 to 0f13518 Compare August 6, 2024 12:38
@Lms24 Lms24 self-assigned this Aug 6, 2024
@Lms24 Lms24 marked this pull request as ready for review August 6, 2024 12:38
@Lms24 Lms24 requested review from a team, mydea and andreiborza and removed request for a team August 6, 2024 12:38
@@ -16,8 +16,7 @@ describe('getTraceMetaTags', () => {
baggage: 'sentry-environment=production',
});

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
// @ts-expect-error - response is defined, types just don't reflect it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait but is this not introducing a ts-expect-error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol thx good catch! I guess I changed it back to expect-error to test the rule and the rule was incorrect so it actually still passed 🤦‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, I adjusted the rule, now it actually throws when you use @ts-expect-error

@Lms24 Lms24 merged commit 69d4823 into develop Aug 6, 2024
123 checks passed
@Lms24 Lms24 deleted the lms/chore-lint-tsignore-node-integ-tests branch August 6, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants